Skip to content

Conversation

@gpunto
Copy link
Contributor

@gpunto gpunto commented Nov 12, 2025

Goal

We're not fully clearing the state on feed deleted, but we should.

Implementation

Clear state that we were not clearing.

Testing

Not sure how feasible this is to do manually, but you'd have to observe all properties of a Feed, then delete it and verify they're all reset.

Checklist

  • Issue linked (if any)
  • Tests/docs updated
  • I have signed the Stream CLA (required for external contributors)

@gpunto gpunto requested a review from Copilot November 12, 2025 15:50
@gpunto gpunto added the pr:bug Bug fix label Nov 12, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 12, 2025

PR checklist ✅

All required conditions are satisfied:

  • Title length is OK (or ignored by label).
  • At least one pr: label exists.
  • Sections ### Goal, ### Implementation, and ### Testing are filled.

🎉 Great job! This PR is ready for review.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 12, 2025

SDK Size Comparison 📏

SDK Before After Difference Status
stream-feeds-android-client 2.36 MB 2.36 MB 0.00 MB 🟢

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes an issue where the feed state was not being completely cleared when a feed is deleted, ensuring all stateful properties are properly reset.

Key Changes:

  • Added clearing of aggregatedActivities, pinnedActivities, notificationStatus, and activitiesPagination in the onFeedDeleted() method
  • Updated the test to verify these additional state properties are cleared
  • Changed test assertions from assertEquals(emptyList<T>(), ...) to assertTrue(...isEmpty()) for cleaner readability

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
FeedStateImpl.kt Added clearing of previously missed state properties (aggregatedActivities, pinnedActivities, notificationStatus, activitiesPagination) in the onFeedDeleted() method
FeedStateImplTest.kt Enhanced test coverage for onFeedDeleted() to verify all state properties are cleared, including the newly added ones

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@gpunto gpunto force-pushed the fully-clear-fid-state-on-deletion branch from 80b3048 to ffc88a6 Compare November 13, 2025 10:08
@gpunto gpunto marked this pull request as ready for review November 13, 2025 10:08
@sonarqubecloud
Copy link

@VelikovPetar VelikovPetar merged commit 29be2a5 into develop Nov 14, 2025
7 checks passed
@VelikovPetar VelikovPetar deleted the fully-clear-fid-state-on-deletion branch November 14, 2025 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants